Skip to content

feat: add as_generic_chat_history function to convert any Context to …#1007

Open
akihikokuroda wants to merge 5 commits intogenerative-computing:mainfrom
akihikokuroda:issue932
Open

feat: add as_generic_chat_history function to convert any Context to …#1007
akihikokuroda wants to merge 5 commits intogenerative-computing:mainfrom
akihikokuroda:issue932

Conversation

@akihikokuroda
Copy link
Copy Markdown
Member

@akihikokuroda akihikokuroda commented May 4, 2026

Misc PR

Type of PR

  • Bug Fix
  • New Feature
  • Documentation
  • Other

Description

Add as_generic_chat_history() function to handle heterogeneous contexts with
configurable formatter. Unlike as_chat_history(), this gracefully handles
arbitrary component types by converting them to strings, with optional logging
for unknown types.

  • as_generic_chat_history(ctx, formatter=None) converts any Context to Messages
  • Default formatter logs warnings for unknown component types
  • Support custom formatter parameter for domain-specific type handling
  • Handle ModelOutputThunk, CBlock, and arbitrary components seamlessly
  • CBlock → Message(role="user"), MOT → Message(role="assistant")
  • Add 7 comprehensive tests covering all conversion scenarios

Testing

  • Tests added to the respective file if code was changed
  • New code has 100% coverage if code as added
  • Ensure existing tests and github automation passes (a maintainer will kick off the github automation when the rest of the PR is populated)

Attribution

  • AI coding assistants used: claude

…Message

Signed-off-by: Akihiko Kuroda <akihikokuroda2020@gmail.com>
@akihikokuroda akihikokuroda requested a review from a team as a code owner May 4, 2026 18:59
@akihikokuroda akihikokuroda requested review from markstur and planetf1 May 4, 2026 18:59
@github-actions github-actions Bot added the enhancement New feature or request label May 4, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

The PR description has been updated. Please fill out the template for your PR to be reviewed.

Comment thread mellea/stdlib/components/chat.py Outdated
return c.parsed_repr
# Use value if parsed_repr is None or not a Message
content = (
str(c.value) if c.parsed_repr is None else formatter(c.parsed_repr)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we also need to consider the case if c.parsed_repr is a string (I believe that's what a MOT will return after a completed CBlock action (?) )

quick way to test:

mot = ModelOutputThunk(value='reply text')
mot.parsed_repr = 'reply text'
as_generic_chat_history(ChatContext().add(Message('user','hi')).add(mot))
# → WARNING: Unknown component type str ...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it was an issue. I fix it. Thanks!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we add a test case for it too?

Signed-off-by: Akihiko Kuroda <akihikokuroda2020@gmail.com>
@psschwei
Copy link
Copy Markdown
Member

psschwei commented May 5, 2026

there's a typo in the PR title: s/ ant / any /

@akihikokuroda akihikokuroda changed the title feat: add as_generic_chat_history function to convert ant Context to … feat: add as_generic_chat_history function to convert any Context to … May 5, 2026
Signed-off-by: Akihiko Kuroda <akihikokuroda2020@gmail.com>

def as_generic_chat_history(
ctx: Context, formatter: Callable[[Any], str] | None = None
) -> list[Message]:
Copy link
Copy Markdown
Contributor

@planetf1 planetf1 May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor / style: Small typing issue here — both _default_formatter and the formatter parameter use Any, which the project convention asks us to avoid unless an external library forces it (nothing does here). object is the right type: it accepts everything, is covariant, and keeps mypy happy with callers that pass typed formatters.

def _default_formatter(obj: object) -> str: ...
formatter: Callable[[object], str] | None = None

The test helpers already use obj: object, so it is just the library code that needs updating.

Comment thread mellea/stdlib/components/chat.py Outdated
# Use value if parsed_repr is None or some other type
content = (
str(c.value) if c.parsed_repr is None else formatter(c.parsed_repr)
)
Copy link
Copy Markdown
Contributor

@planetf1 planetf1 May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two things worth noting on this line — one minor, one more significant.

Minor / style: When parsed_repr is a dict or similar, the default formatter will log "Unknown component type dict" — but dict is not an unexpected component here, it is a parsed form of a ModelOutputThunk we already recognise. That message will be confusing to anyone monitoring logs. A dedicated log line would be clearer:

_logger.warning(
    "ModelOutputThunk.parsed_repr is %s, not a Message; falling back to value.",
    type(c.parsed_repr).__name__,
)

More significant: If both parsed_repr and value are None (a ModelOutputThunk that has not been evaluated yet), str(None) silently produces the literal string "None" in the chat history with no warning logged. A backend receiving that will have no indication anything went wrong. Worth guarding explicitly:

if c.value is None:
    raise ValueError("ModelOutputThunk has no value and no parsed_repr — was it evaluated?")

return Message(role="assistant", content=content)
case CBlock():
return Message(role="user", content=str(c))
case _:
Copy link
Copy Markdown
Contributor

@planetf1 planetf1 May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More significant: One tricky edge case: ImageBlock inherits from CBlock (see core/base.py:79), so it will fall into this arm and produce Message(role="user", content=str(image_block)) — meaning a raw base64 PNG string ends up in the chat history as a text message. This is a reachable code path if a context with image content is passed in, and the corruption would be invisible downstream.

A simple guard fixes it:

case CBlock():
    if type(c) is not CBlock:
        return Message(role="user", content=formatter(c))
    return Message(role="user", content=str(c))

That way any CBlock subclasses not explicitly handled go through the formatter instead.

Copy link
Copy Markdown
Contributor

@planetf1 planetf1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks good - just came across 2 possible corruption issues. 1 is an edge case, the other affects images. 2 other minors.

Signed-off-by: Akihiko Kuroda <akihikokuroda2020@gmail.com>
@akihikokuroda akihikokuroda requested review from planetf1 and psschwei May 6, 2026 13:24
Comment thread mellea/stdlib/components/chat.py Outdated
Co-authored-by: Paul Schweigert <paul@paulschweigert.com>
@akihikokuroda akihikokuroda requested a review from psschwei May 6, 2026 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: as_chat_history is outdated and functionality doesn't match it's name

3 participants